Conversation
rigoucr
changed the title
|
@codex please reviews |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61e7ca316d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - approve-playwright-vrt-test: | ||
| type: approval | ||
| requires: | ||
| - deploy_to_pantheon |
There was a problem hiding this comment.
approve-playwright-vrt-test is always added to build-deploy-pantheon, even though run-playwright-vrt-tests defaults to false. CircleCI approval jobs pause the workflow until someone approves or cancels them, so Pantheon PR builds that are not using VRT will still sit in Needs approval/pending instead of reaching a terminal success state; that is a bad default for an opt-in job.
Useful? React with 👍 / 👎.
| - approve_deploy_to_test: | ||
| type: approval | ||
| requires: |
There was a problem hiding this comment.
On main/master, approve_deploy_to_test only requires deploy_to_pantheon, so a user can approve promotion as soon as the multidev deploy finishes. If playwright-e2e-test is still running or has failed (and likewise playwright-vrt-test when enabled), CircleCI will not block the test/live promotion because those jobs are not listed in requires, which defeats the new pre-deploy validation step.
Useful? React with 👍 / 👎.
| docker: | ||
| - image: quay.io/pantheon-public/build-tools-ci:8.x-php<< pipeline.parameters.php-version >> | ||
| executor: playwright-executor | ||
| parallelism: 4 |
There was a problem hiding this comment.
This job is fanned out with parallelism: 4, but the test command later in the job is still a plain npx playwright test --grep-invert "@vrt" (and playwright-vrt-test repeats the same pattern with npm run vrt:ci). CircleCI's parallelism docs and Playwright's sharding docs both require an explicit shard/split command here; otherwise each executor reruns the entire suite, so enabling these jobs multiplies runtime and credits by four instead of speeding the tests up.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0caebdacb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| docker: | ||
| - image: quay.io/pantheon-public/build-tools-ci:8.x-php<< pipeline.parameters.php-version >> | ||
| executor: playwright-executor |
There was a problem hiding this comment.
playwright-e2e-test defines both a docker stanza and an executor, but CircleCI requires exactly one executor type per job. This makes the config invalid at compile/validation time, so the workflow cannot start; the same mixed definition is repeated in playwright-vrt-test, so fixing only one job is insufficient.
Useful? React with 👍 / 👎.
| fi | ||
| - run: | | ||
| if [ -n "$BASIC_USER" ] && [ -n "$BASIC_PASS" ]; then | ||
| echo "export REMOTE_ENV_BASE_URL=https://${BASIC_USER}:${BASIC_PASS}@${TERMINUS_ENV}-${TERMINUS_SITE}.pantheonsite.io/" >> $BASH_ENV |
There was a problem hiding this comment.
When BASIC_USER/BASIC_PASS are set, this builds REMOTE_ENV_BASE_URL by embedding credentials directly in the URL. Any reserved character in the password (for example @, :, /, #, %) can corrupt URL parsing and cause the curl wake-up step to fail, which blocks E2E execution on password-protected environments.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12814548ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| This boolean controls whether the Playwright end-to-end test job runs after the | ||
| Pantheon multidev is created. | ||
|
|
||
| Default: `true` |
There was a problem hiding this comment.
README.MD says run-playwright-e2e-tests and run-playwright-vrt-tests default to true and playwright-vrt-target-env defaults to "test", but config.yml sets those defaults to false, false, and "live" (lines 24, 29, 33). This mismatch causes operators to assume checks are running automatically and comparing against test when they are actually skipped or targeting live, which can lead to missed validation or confusing VRT behavior.
Useful? React with 👍 / 👎.
1 participant
https://app.clickup.com/t/36718269/4K_INT-43